Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add script in the repo to setup envtest #1608

Merged
merged 2 commits into from
Jul 23, 2020
Merged

feat: add script in the repo to setup envtest #1608

merged 2 commits into from
Jul 23, 2020

Conversation

camilamacedo86
Copy link
Member

@camilamacedo86 camilamacedo86 commented Jul 23, 2020 •
edited
Loading

This pr is just for we add the script that will be used in the solution to set up the env test locally for we are able to close #1599. See:

camilamacedo@Camilas-MacBook-Pro ~/go/src/sigs.k8s.io/kubebuilder (only-script) $ rm -rf testbin/
camilamacedo@Camilas-MacBook-Pro ~/go/src/sigs.k8s.io/kubebuilder (only-script) $ ./scripts/setup_envtest_bins.sh v1.18.2 v3.4.3
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 26.6M  100 26.6M    0     0  6759k      0  0:00:04  0:00:04 --:--:-- 6760k
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 47.8M  100 47.8M    0     0  7932k      0  0:00:06  0:00:06 --:--:-- 8369k
./testbin/kube-apiserver: replacing existing signature
./testbin/kube-apiserver: signed Mach-O thin (x86_64) [kube-apiserver-b9be6305b853bc12b0bf0ab6cf1b219fc4927cad]

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 23, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 23, 2020
@camilamacedo86
Copy link
Member Author

/assign @pwittrock

pwittrock
pwittrock reviewed Jul 23, 2020
View reviewed changes
scripts/setup_envtest_bins.sh Outdated
# In this way, to have the kube-apiserver is required to build it locally
# if the project is cloned locally already do nothing
if [ ! -d $GOPATH/src/k8s.io/kubernetes ]; then
git clone https://github.com/kubernetes/kubernetes $GOPATH/src/k8s.io/kubernetes --depth=1 -b v1.18.2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this use the K8S_VER env var?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catcher.

estroz
estroz reviewed Jul 23, 2020
View reviewed changes
scripts/setup_envtest_bins.sh Outdated
# To use envtest is required to have etcd, kube-apiserver and kubetcl binaries installed locally.
# This script will create the directory testbin and perform this setup for linux or mac os x envs in

K8S_VER=v1.18.2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value should be a script parameter ideally.

pwittrock
pwittrock reviewed Jul 23, 2020
View reviewed changes
scripts/setup_envtest_bins.sh
# This script will create the directory testbin and perform this setup for linux or mac os x envs in

# Kubernetes version e.g v1.18.2
K8S_VER=$1
Copy link
Contributor

@pwittrock pwittrock Jul 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow up consider defaulting these if they are unset. Maybe keep as env var

@pwittrock
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 23, 2020
@k8s-ci-robot k8s-ci-robot merged commit a7be4b7 into kubernetes-sigs:master Jul 23, 2020
@camilamacedo86 camilamacedo86 deleted the only-script branch July 23, 2020 18:37
estroz
estroz reviewed Jul 23, 2020
View reviewed changes
Copy link
Contributor

@estroz estroz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs documentation @camilamacedo86.

@camilamacedo86
Copy link
Member Author

camilamacedo86 commented Jul 23, 2020 •
edited
Loading

@estroz I still think that we need the new target in the Makefile that will call this script as you suggested.
See that users will no longer able to perform the quick start steps when the bins are no longer being shipped with kb. #686
We need to disengage the scaffold from the bins in kb as we need to solve this scenario when the installation is made via master and not the bin. See #1599. So, the docs will be done in this follow-up. We need to have the link for the script merged to continue on it.

@estroz
Copy link
Contributor

estroz commented Jul 23, 2020 •
edited
Loading

Users shouldn't have to download kubebuilder's source to get this file. Its download and use should be documented in the quickstart. See #1600 (review).

Edit: I may be misunderstanding the problem this script is trying to solve. I did not realize upstream server binaries were being packaged with the kubebuilder binary.

@camilamacedo86 camilamacedo86 mentioned this pull request Jul 24, 2020
Merged
@camilamacedo86 camilamacedo86 mentioned this pull request Aug 5, 2020
Merged
@camilamacedo86 camilamacedo86 mentioned this pull request Feb 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estroz estroz estroz left review comments

@pwittrock pwittrock pwittrock left review comments

@DirectXMan12 DirectXMan12 Awaiting requested review from DirectXMan12

Assignees

@pwittrock pwittrock

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

unable to run the tests scaffold for the projects without the kb bin in /usr/local/kubebuilder/bin
4 participants
@camilamacedo86 @k8s-ci-robot @pwittrock @estroz